This seems like the one place where we need a goBack method in the site editor's usePostLinkProps, and only when you have navigated from a page to edit its template. Still trying to figure out how best to handle this 🤔

I wonder if the issue might be the code deleted on line 82-85, which seems important and maybe shouldn't be deleted. The defaultRenderingMode is one of template-locked or all, so not related to template-only.

Also noticing that currently when you edit a page, all blocks seems to be editable, compared to in trunk where only the post content is editable.

I wonder if the issue might be the code deleted on line 82-85, which seems important and maybe shouldn't be deleted.

Because we are loading a new entity into the editor now on going back the default rendering mode will be set on the new editor load so I don't think we also need to do it here.

Also noticing that currently when you edit a page, all blocks seems to be editable, compared to in trunk where only the post content is editable.

This was caused by the way I was setting the default rendering mode - should be fixed now.

Yes, the goBack method shouldn't passed to the site editor provider unless we actually went into focus mode, so the site editor need to be a second history of focused entities (in addition to the url history) kind of similar to the post editor. And navigating in the site editor outside these methods should reset the focused entities history.

This was caused by the way I was setting the default rendering mode - should be fixed now.

Hmm, still doesn't seem to be right, the page content doesn't show 🤔

for me goBack just means isFocusMode which translates to:

• Having a callback to go back to the previous focus entity or parent entity.
• Showing a padding around the canvas.

So we need in two places.

The other thing is, I'd like to arrive to a point in the next months where we could render the whole editor using <Editor post={post} settings={settings) /> to render the whole editor. You won't use the editor components one by one, which means at some point we need to address these specific components passed to specific components (probably become settings in most cases if really needed or just go away). That said, adding props to these components is fine temporarily (especially if these components are private APIs)

Hmm, still doesn't seem to be right, the page content doesn't show 🤔

This is hopefully fixed by this @talldan

Adding a goBack option in the site editor is a little more complex because it has a router and the back button works. At a quick glance it looks like we would need to add something like a popstate eventlistener and try and work out if we are coming back from a focus mode page in order to clear the goBack setting.

There may be an easier way, but I would like to suggest we keep it simple for this PR and just use the existing focusMode param that is used already in the site editor to get the back link (using the router back method) and the editor padding. This change does this and it seems to work, and we can always revisit this in a follow up if we do want to tie it in closer with the router.

While it's a follow-up to a follow-up, making it another follow-up seems ok to me. This PR is getting quite big and comment-y, so breaking it out into a separate PR would make it easier to review.

yeh, isFocusMode has been in the code base for at least 6 months, and this PR is intended to remove the recently added template-only mode before 6.5, so I think it makes sense rather than doing a follow-up directly after this to instead look at removing isFocusedMode as a separate task post 6.5, at which point we will have more time to plan and test more complex routing changes.

I wonder if there's a way to merge "all" and "post-only" mode, what's the remaining difference between these two modes?

post-only is still quite different to all, eg. with post-only the theme layout width, etc. are applied to the full post, page title displays larger, etc:

post-only:

all:

We may be able to refactor this somehow to get rid of the need for these modes, but I think we should limit this PR to template-only and revisit the other modes post 6.5.

I really don't like the name of the setting. I understand that it's needed because of the "href" but how important is it to have both the href and the onClick, onSelectEntityRecord would have been way better (or something like that).

I agree. The naming was ok in the context of editing the synced patterns, but it hasn't extended well to this wider use case. As well as the setting naming, the onClick property is now also being used in places that don't involve a click.

I opened a PR here with some suggested renaming options, initially getEntityLoader, but @talldan thought the use of entity might be confused with the core-data apis.

That PR currently has the setting named as getPostNavigation, but the use of post is also not the best as we are using it to load templates and patterns. What about getEntityNavigation with properties of goTo and linkTo ?

What I don't like is the "get" prefix, I prefer a callback directly "on" something, so my question is can't we get rid of the href and just leave the onClick?

I actually think href is more important than onClick in terms of universal routing. In a future context, we might be able to just use regular <a> tags and href attributes for the post editor or shared packages (core-commands). In the site editor, we can attach an event listener globally that listens to all the bubbled events with the <a> links. Then, we can intercept the click and use history.push() under the hood to achieve seamless client-side routing.

href provides more context in this case, and it's accessible from the browsers' viewpoint as well (the bottom-left address when you hover over it and open-in-new-tab functionality, for instance.)

I actually think href is more important than onClick in terms of universal routing.

I agree when it comes to links. The problem is that this is an API of the "block-editor" package and these actions are not always links. If I'm not wrong, there's no href in the post editor for instance, so it's highly dependent on the context. We're going to add an API that is not suited for all use-cases and forward compatibility might come and bite us later.

The href was initially working in the post editor when we were updating the url params, but that caused other issues, so href has been broken in the post editor since we stopped updating the url. So, I have removed the href and renamed the setting to your suggestion @youknowriad of onSelectEntityRecord. There is a PR for this here.

I haven't extensively tested this yet, I will wait and see if you are happy with this new naming approach first - so don't spend too much time testing/reviewing, if you can let me know if this is along the lines of what you are thinking I will then give it a proper test and move it from draft.

I believe getPostLinkProps is probably a function that shouldn't come from link.js file (which in my mind should't have "post" in it), maybe it's the name of the function that is wrong.

Yeh, I think this ties in with the naming issue mentioned here. Given you approved this one I will assume you are happy to fix that in a follow-up rename PR. What did you think of getEntityNavigation ?

I think it should move to the current file rather than stay in link basically, regardless of its name. That said, that's very very minor.